Conversation
maryliag
requested review from
a team,
alolita,
austinlparker,
jpkrohling,
mtwo,
mx-psi,
svrnm,
tedsuo and
trask
as code owners
|
Results of the script: (latests itaration) INACTIVE MEMBERS REPORT
|
|
Besides the repo I've already excluded, I'm considering removing the following ones I would like a feedback to confirm if make sense:
|
|
For I'd like to suggest evaluating activity for an individual within any repository their group is responsible, and then we can also evaluate, separately, aggregate activity for a group in a repository. Regarding initial results though - this is good data to have. I have been putting off going to emiritus status for C++ and Java approver, but given the current intensity of semantic conventions work I just don't think I'll be able to contribute meaningfully there as an approver. |
Could you expand on this ? Where does "the SIG is shutting down" comes from ? |
This repository is very low traffic, but is it not dead: it is stable, and deliverables there (docker images) are used daily. Tooling very rarely needs to be adjusted, hence the lack of activity. |
From here: #3297 |
That is what is doing already, so if a user is part of team X and that team has 5 repos to it, if they're active into any of those 5, they're consider active in general and I don't tag that into any of the 5 repos "assigned" to that team. The example you provided, that user is not marked as active in the weaver repo as you mentioned. You can actually see that user got flagged as inactive into the 3 repos the team
I'm glad this is already helping :D |
|
Thanks for the data, it's really useful! A few questions:
|
|
Thanks for sharing the data, this is helpful. For I was also one of the early contributors to the C++ SIG and involved in the initial implementation of several signals, so I have historical context around parts of the implementation that can still be helpful when questions or design discussions come up. We also actively use otel-cpp within Microsoft and plan to continue doing so, so it would be good for me to remain on the maintainer list for those repos. |
I have some questions on this. Having a report to see activity in general and raise flags is one thing, and is good. The question then is what to do with the results. I am concerned with the workflow and how automated this will be, possibly locking everyone out of a given repository just because not much happens there. Even if there is a workflow that files PR automatically, I assume maintainers will have the final decision on what to do, correct ? |
|
A technical comment here: When someone is part of x-maintainers and x-approvers, any activity such as:
will NOT count as maintainer activity, it will be reported as approver activity for code reviews, or not at all. The only activity that will register as a maintainer task is to simply click on the merge button (and maybe make a release ?). A workflow that starts to evict maintainers who otherwise participate in a repository, but just do not perform merges, is dangerous in my opinion, activity should be evaluated globally. Edit: This is based on my understanding of github privileges and when the maintainer priviledge is checked, this is not based on the script logic to check maintainer activity. |
|
One observation on the maintainer activity check in the script: it looks like the current logic requires authored PRs in addition to reviews/comments in order to consider a maintainer active. That might be a bit strict. According to the maintainer role description, activity can also show up through reviewing proposals and PRs, contributing to discussions, helping with design direction, etc. In more mature or stable repositories, maintainers may stay active mostly through review and guidance rather than regularly authoring PRs. So requiring authored PRs as a mandatory signal may undercount maintainers who are still actively involved in the project. |
I think maybe we should check the script? Laurent just helped review our release and a few other tidbits on friday, is active in our weekly SIG and I believe has reviewed PRs within the bounds of your script (past 2 weeks). I'm actually surprised to see his name on the list for weaver and its associated repositories then, apologies I missed it was also on the otel/weaver. |
|
Thank you all for the comments! First thing, this is a script based on the proposal displayed here. That is the issue that discuss what you need to do to be consider active in each role, but I also want to acknowledge that are case by case scenarios that need changes, this is why I created the example of a result and shared here, so we can analyze these cases and make the proper updates (to the proposal and/or script). Now, let me answer the questions/comments: From @lmolkova:
From @lalitb:
From @marcalff
From @jsuereth
Hope this info has helped! 😅 |
|
The first comment is updated to the latest result after script changes (ignoring new repos, and removing If there is an agreement, I can also add |
|
Updated both the script and the proposal, so the maintainer condition is now an OR and not an AND. Also updated the first comment with the latest results |
|
Updates the script so now that comments on issues count for approver and maintainer |
|
@maryliag I'm surprised to not have python-contrib there, could you please double check that it has been accounted for? |
I can see that the 2 teams related to python-contrib that were checked were: |
|
I was able to find a scenario of false positive for users being active, when they shouldn't be. Updated once more, also the script has a debug options to help identify cases. Thank you @xrmx for pointing it out an unexpected result. |
|
PR updated to generate PRs. Example of a PR creation can be found here Ready for final review and any feedback of repos or teams that should be added to the ignore list here |
pellared
left a comment
pellared
left a comment
There was a problem hiding this comment.
I wish having unit tests here 😉
|
My role shifted but to non-OTel stuff for some time, but it just shifted back. I will likely be making some changes regarding genai instrumentations soon. Please don't remove my status yet. |
|
I work on otel stuff every day in my role at elastic. Just haven't been contributing upstream. happy to do so more if that helps |
it's hard because there are so many edge cases that we will probably going to catch only with real data. But people can always tag me if adjustments need to be made 🙏 |
|
@jeremydvoss, @ericmustin & everyone that is messaging me asking to not be removed: This PR is not removing anyone yet. This is showing the example of results and once merged it will be executed monthly, and that will then create a PR to the respective repos for people flagged as inactive to be removed and it's up to the maintainers of each repo to approve the change or not. Once that PR is open, you can discuss with the maintainers of the repo if you should be removed. Keep in mind that we want to show a realistic state of each SIG, so if you're not contributing but have status it might give the wrong impression that the repo has a higher amount of active maintainers/approver/triagers than it actually have, and that causes problem for the SIG health. Reach out to the maintainers of your SIG on how you can better contribute, and don't forget that each role has a set of responsibilities and that you can be removed if you're not doing them: Hope that helps to clarify! |
|
Just waiting on some feedback if the following repos should be added to the ignore list: opentelemetry-proto* The way the script is now is hard to identify for low traffic repos, so a following update could be checking specific cases, per folder/ownership for these particular repos. So I'm happy to remove them for now if people think it make sense and don't keep creating false positives. |
I would rather remove them from the list and start smaller for now |
11 participants
This is analyzing which members are no longer active according to the rules, and then generating the final list. If for a particular repo, a user is part of multiple teams (x-triager, x-approver) this is listing all those teams, so it's clear they should be removed from both.
There are a few repos excluded (since it doesn't make sense), similar to particular teams and repos combination.
The first comment will add the result of this script, this way I would like to get a review if there are any other specific exceptions that need to be added.
The script also generates a PR on each respective repo with the updates on the readme file. The maintainers still need to manually remove them from the teams. The workflow in this PR executes the script monthly. Examples of a PR creation can be found:
For now, this is handling triagers, approvers and maintainers.
This script is based on the proposal found here: #3295